Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extract PDF bookmark with priority #66

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

extract PDF bookmark with priority #66

wants to merge 3 commits into from

Conversation

bjsi
Copy link
Member

@bjsi bjsi commented Feb 16, 2020

Allows you to extract a bookmark from a PDF and specify the priority in a popup window.

  1. Select bookmark

image

  1. Press Ctrl-Shift-X and type the priority

image

  1. Bookmark gets extracted

image

There's also a menu item:

image

@bjsi bjsi linked an issue Feb 16, 2020 that may be closed by this pull request
@alexis- alexis- self-requested a review March 12, 2020 12:32
Copy link
Member

@alexis- alexis- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some minor issues to fix.

{
PDFElement pdfEl;
string title;
string author;
string creationDate;
string filePath;


if (priority < 0 || priority > 100)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. Can you also add a LogTo.Error(...) message since this should never happen ? That way a bug report will be sent if it does happen

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok sorted.

return;
}

IPDFViewer.ExtractBookmark(bookmark, result.Model.Value);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a check for the Priority value provided by the user, like in your other Extract with priority PR ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I added a check and included an Alert window if the priority value is invalid.

@@ -225,7 +225,7 @@ protected ContentBase CreateImageContent(Image image, string title)

// PDF Extracts

protected bool CreatePDFExtract()
protected bool CreatePDFExtract(double priority = -1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is priority never used ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I think I added that by mistake. I removed it in the latest commit.

@alexis- alexis- added this to the SMA 2.1.0 "Open-Beta" milestone Mar 12, 2020
@alexis- alexis- added the enhancement New feature or request label Mar 12, 2020
@alexis-
Copy link
Member

alexis- commented Aug 3, 2020

Looks like there are some conflicts. Could you take a look whenever you have a moment ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

Ability to bookmark extract (ctrl+shift+x) with priority determined
2 participants